Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cli] Several changes #1428

Merged
merged 3 commits into from
Nov 7, 2019
Merged

[cli] Several changes #1428

merged 3 commits into from
Nov 7, 2019

Conversation

mcortesi
Copy link
Contributor

@mcortesi mcortesi commented Oct 22, 2019

Description

Changes:

  • validator deregister.
  • Split affliliate/deaffiliate
  • UX: Add checks before doing a tx
  • Test: Support for testing commands
  • Add a few tests (we need much more)
  • Add Support to call validator action using authorized signer

Tested

Related issues

Backwards compatibility

Changes cli affiliate command

@codecov
Copy link

codecov bot commented Oct 23, 2019

Codecov Report

Merging #1428 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1428   +/-   ##
=======================================
  Coverage   74.15%   74.15%           
=======================================
  Files         287      287           
  Lines        7652     7652           
  Branches      667      667           
=======================================
  Hits         5674     5674           
  Misses       1865     1865           
  Partials      113      113
Flag Coverage Δ
#mobile 74.15% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 730c950...63cf911. Read the comment docs.

Copy link
Contributor

@asaj asaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great

packages/cli/src/test-utils/ganache-test.ts Show resolved Hide resolved
packages/cli/src/utils/checks.ts Show resolved Hide resolved
* @param signer Address that is authorized to sign the tx as validator
* @return The Account address
*/
validatorSignerToAccount: (signer: Address) => Promise<Address> = proxyCall(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this consistent with what @nambrot is renaming things? If not, can we aim for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wanted to be on top of nam pr but couldn't...
so when we have that, we'll have to move some methods and have a consistent naming

* @param address The address of the account
* @return Returns `true` if account exists. Returns `false` otherwise.
*/
isSigner: (address: string) => Promise<boolean> = proxyCall(this.contract.methods.isAuthorized)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

answered above

@asaj asaj assigned mcortesi and unassigned asaj Oct 24, 2019
@mcortesi mcortesi changed the title [cli] validator deregister. Split affliliate/deaffiliate [cli] Several changes Oct 24, 2019
@asaj asaj assigned asaj and unassigned mcortesi Oct 24, 2019
@nategraf nategraf removed their assignment Oct 25, 2019
Copy link
Contributor

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


process.env.NO_SYNCCHECK = 'true'

testWithGanache('account:authorize cmd', (web3: Web3) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!

@@ -1,30 +1,30 @@
import { flags } from '@oclif/command'
import { CLIError } from '@oclif/errors'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@mcortesi mcortesi changed the base branch from asaj/pos-2 to master November 7, 2019 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validators should be able to deaffiliate through the CLI
4 participants